Skip to content

Conversation

@MrCreosote
Copy link
Member

Next will add a cli command that runs it

Next will add a cli command that runs it
@MrCreosote MrCreosote requested a review from briehl October 13, 2025 05:51
@codecov
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.16%. Comparing base (2c205ad) to head (84665b1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../java/us/kbase/sdk/util/DeployConfigGenerator.java 98.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #176      +/-   ##
============================================
+ Coverage     71.80%   72.16%   +0.35%     
- Complexity      691      705      +14     
============================================
  Files            46       47       +1     
  Lines          3721     3768      +47     
  Branches        657      664       +7     
============================================
+ Hits           2672     2719      +47     
  Misses          815      815              
  Partials        234      234              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Tests require it. Not sure whether it's actually used or not, so safe
side for now
UJS no longer exists

the njs_wrapper endpoint doesn't exist either, but not sure if that's
used to pass the ee2 url
shockUrl = getConfigUrl(props, "shock_url", endPoint, "shock-api");
handleUrl = getConfigUrl(props, "handle_url", endPoint, "handle_service");
srvWizUrl = getConfigUrl(props, "srv_wiz_url", endPoint, "service_wizard");
// not sure if this is still used for the ee2 url or not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes. I hope not!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, sure, leave it in for now. Do we need to do some kind of audit to find out?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's harmful to leave it for now. I'm not planning to do anything unless it becomes an issue

Comment on lines +43 to +44
// for now. We might want to be more stringent about the input vs. silently accepting
// whatever.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@MrCreosote MrCreosote merged commit 8bfcad7 into main Oct 20, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants